PushoverAgent: Treat parameter options as templates rather than default values

This should fix #1718.

While at it, character limit implementation is fixed: `slice(0..n)`
would take the first n+1 characters instead of n.

Akinori MUSHA 7 years ago
parent
commit
a47c5c8a80

+ 51 - 47
app/models/agents/pushover_agent.rb

@@ -12,42 +12,41 @@ module Agents
12 12
 
13 13
       **You need a Pushover API Token:** [https://pushover.net/apps/build](https://pushover.net/apps/build)
14 14
 
15
-      **You must provide** a `message` or `text` key that will contain the body of the notification. This can come from an event or be set as a default. Pushover API has a `512` Character Limit including `title`. `message` will be truncated.
16
-
17 15
       * `token`: your application's API token
18 16
       * `user`: the user or group key (not e-mail address).
19 17
       * `expected_receive_period_in_days`:  is maximum number of days that you would expect to pass between events being received by this agent.
20 18
 
21
-      Your event can provide any of the following optional parameters or you can provide defaults:
19
+      The following options are all Liquid templates which evaluated values will be posted to Pushover API.  Only the `message` parameter is required, and if it is blank API call is omitted.
20
+
21
+      Pushover API has a `512` Character Limit including `title`.  `message` will be truncated.
22 22
 
23
+      * `message` - your message (required)
23 24
       * `device` - your user's device name to send the message directly to that device, rather than all of the user's devices
24 25
       * `title` or `subject` - your notification's title
25 26
       * `url` - a supplementary URL to show with your message - `512` Character Limit
26 27
       * `url_title` - a title for your supplementary URL, otherwise just the URL is shown - `100` Character Limit
28
+      * `timestamp` - a [Unix timestamp](https://en.wikipedia.org/wiki/Unix_time) of your message's date and time to display to the user, rather than the time your message is received by the Pushover API.
27 29
       * `priority` - send as `-1` to always send as a quiet notification, `0` is default, `1` to display as high-priority and bypass the user's quiet hours, or `2` for emergency priority: [Please read Pushover Docs on Emergency Priority](https://pushover.net/api#priority)
28 30
       * `sound` - the name of one of the sounds supported by device clients to override the user's default sound choice. [See PushOver docs for sound options.](https://pushover.net/api#sounds)
29 31
       * `retry` - Required for emergency priority - Specifies how often (in seconds) the Pushover servers will send the same notification to the user. Minimum value: `30`
30 32
       * `expire` - Required for emergency priority - Specifies how many seconds your notification will continue to be retried for (every retry seconds). Maximum value: `86400`
31 33
 
32
-      Your event can also pass along a timestamp parameter:
33
-
34
-      * `timestamp` - a [Unix timestamp](https://en.wikipedia.org/wiki/Unix_time) of your message's date and time to display to the user, rather than the time your message is received by the Pushover API.
35
-
36 34
     MD
37 35
 
38 36
     def default_options
39 37
       {
40 38
         'token' => '',
41 39
         'user' => '',
42
-        'message' => 'a default message',
43
-        'device' => '',
44
-        'title' => '',
45
-        'url' => '',
46
-        'url_title' => '',
47
-        'priority' => '0',
48
-        'sound' => 'pushover',
49
-        'retry' => '0',
50
-        'expire' => '0',
40
+        'message' => '{{ message }}',
41
+        'device' => '{{ device }}',
42
+        'title' => '{{ title }}',
43
+        'url' => '{{ url }}',
44
+        'url_title' => '{{ url_title }}',
45
+        'priority' => '{{ priority }}',
46
+        'timestamp' => '{{ timestamp }}',
47
+        'sound' => '{{ sound }}',
48
+        'retry' => '{{ retry }}',
49
+        'expire' => '{{ expire }}',
51 50
         'expected_receive_period_in_days' => '1'
52 51
       }
53 52
     end
@@ -60,38 +59,43 @@ module Agents
60 59
 
61 60
     def receive(incoming_events)
62 61
       incoming_events.each do |event|
63
-        payload_interpolated = interpolated(event)
64
-        message = (event.payload['message'].presence || event.payload['text'].presence || payload_interpolated['message']).to_s
65
-        if message.present?
66
-          post_params = {
67
-            'token' => payload_interpolated['token'],
68
-            'user' => payload_interpolated['user'],
69
-            'message' => message
70
-          }
71
-
72
-          post_params['device'] = event.payload['device'].presence || payload_interpolated['device']
73
-          post_params['title'] = event.payload['title'].presence || event.payload['subject'].presence || payload_interpolated['title']
74
-
75
-          url = (event.payload['url'].presence || payload_interpolated['url'] || '').to_s
76
-          url = url.slice 0..512
77
-          post_params['url'] = url
78
-
79
-          url_title = (event.payload['url_title'].presence || payload_interpolated['url_title']).to_s
80
-          url_title = url_title.slice 0..100
81
-          post_params['url_title'] = url_title
82
-
83
-          post_params['priority'] = (event.payload['priority'].presence || payload_interpolated['priority']).to_i
84
-
85
-          if event.payload.has_key? 'timestamp'
86
-            post_params['timestamp'] = (event.payload['timestamp']).to_s
62
+        interpolate_with(event) do
63
+          post_params = {}
64
+
65
+          # required parameters
66
+          %w[
67
+            token
68
+            user
69
+            message
70
+          ].all? { |key|
71
+            if value = String.try_convert(interpolated[key].presence)
72
+              post_params[key] = value
73
+            end
74
+          } or next
75
+
76
+          # optional parameters
77
+          %w[
78
+            device
79
+            title
80
+            url
81
+            url_title
82
+            priority
83
+            timestamp
84
+            sound
85
+            retry
86
+            expire
87
+          ].each do |key|
88
+            if value = String.try_convert(interpolated[key].presence)
89
+              case key
90
+              when 'url'
91
+                value.slice!(512..-1)
92
+              when 'url_title'
93
+                value.slice!(100..-1)
94
+              end
95
+              post_params[key] = value
96
+            end
87 97
           end
88 98
 
89
-          post_params['sound'] = (event.payload['sound'].presence || payload_interpolated['sound']).to_s
90
-
91
-          post_params['retry'] = (event.payload['retry'].presence || payload_interpolated['retry']).to_i
92
-
93
-          post_params['expire'] = (event.payload['expire'].presence || payload_interpolated['expire']).to_i
94
-
95 99
           send_notification(post_params)
96 100
         end
97 101
       end
@@ -102,7 +106,7 @@ module Agents
102 106
     end
103 107
 
104 108
     def send_notification(post_params)
105
-      response = HTTParty.post(API_URL, :query => post_params)
109
+      response = HTTParty.post(API_URL, query: post_params)
106 110
       puts response
107 111
     end
108 112
   end

+ 58 - 0
db/migrate/20161004120214_update_pushover_agent_options.rb

@@ -0,0 +1,58 @@
1
+class UpdatePushoverAgentOptions < ActiveRecord::Migration
2
+  DEFAULT_OPTIONS = {
3
+    'message' => '{{ message | default: text }}',
4
+    'device' => '{{ device }}',
5
+    'title' => '{{ title | default: subject }}',
6
+    'url' => '{{ url }}',
7
+    'url_title' => '{{ url_title }}',
8
+    'priority' => '{{ priority }}',
9
+    'timestamp' => '{{ timestamp }}',
10
+    'sound' => '{{ sound }}',
11
+    'retry' => '{{ retry }}',
12
+    'expire' => '{{ expire }}',
13
+  }
14
+
15
+  def up
16
+    Agents::PushoverAgent.find_each do |agent|
17
+      options = agent.options
18
+      DEFAULT_OPTIONS.each_pair do |key, default|
19
+        current = options[key]
20
+
21
+        options[key] =
22
+          if current.blank?
23
+            default
24
+          else
25
+            "#{prefix_for(key)}#{current}#{suffix_for(key)}"
26
+          end
27
+      end
28
+      agent.save!(validate: false)
29
+    end
30
+  end
31
+
32
+  def down
33
+    Agents::PushoverAgent.transaction do
34
+      Agents::PushoverAgent.find_each do |agent|
35
+        options = agent.options
36
+        DEFAULT_OPTIONS.each_pair do |key, default|
37
+          current = options[key]
38
+
39
+          options[key] =
40
+            if current == default
41
+              ''
42
+            else
43
+              current[/\A#{Regexp.quote(prefix_for(key))}(.*)#{Regexp.quote(suffix_for(key))}\z/, 1]
44
+            end or raise ActiveRecord::IrreversibleMigration, "Cannot revert migration once Pushover agents are configured"
45
+        end
46
+        agent.save!(validate: false)
47
+      end
48
+    end
49
+  end
50
+
51
+  def prefix_for(key)
52
+    "{% capture _default_ %}"
53
+  end
54
+
55
+  def suffix_for(key)
56
+    "{% endcapture %}" << DEFAULT_OPTIONS[key].sub(/(?=\}\}\z)/, '| default: _default_ ')
57
+  end
58
+end

+ 34 - 32
spec/models/agents/pushover_agent_spec.rb

@@ -2,27 +2,29 @@ require 'rails_helper'
2 2
 
3 3
 describe Agents::PushoverAgent do
4 4
   before do
5
-    @checker = Agents::PushoverAgent.new(:name => 'Some Name',
6
-                                       :options => { :token => 'x',
7
-                                                :user => 'x',
8
-                                                :message => 'Some Message',
9
-                                                :device => 'Some Device',
10
-                                                :title => 'Some Message Title',
11
-                                                :url => 'http://someurl.com',
12
-                                                :url_title => 'Some Url Title',
13
-                                                :priority => 0,
14
-                                                :timestamp => 'false',
15
-                                                :sound => 'pushover',
16
-                                                :retry => 0,
17
-                                                :expire => 0,
18
-                                                :expected_receive_period_in_days => '1'})
19
-     
5
+    @checker = Agents::PushoverAgent.new(name: 'Some Name',
6
+                                         options: {
7
+                                           token: 'x',
8
+                                           user: 'x',
9
+                                           message: "{{ message | default: text | default: 'Some Message' }}",
10
+                                           device: "{{ device | default: 'Some Device' }}",
11
+                                           title: "{{ title | default: 'Some Message Title' }}",
12
+                                           url: "{{ url | default: 'http://someurl.com' }}",
13
+                                           url_title: "{{ url_title | default: 'Some Url Title' }}",
14
+                                           priority: "{{ priority | default: 0 }}",
15
+                                           timestamp: "{{ timestamp | default: 'false' }}",
16
+                                           sound: "{{ sound | default: 'pushover' }}",
17
+                                           retry: "{{ retry | default: 0 }}",
18
+                                           expire: "{{ expire | default: 0 }}",
19
+                                           expected_receive_period_in_days: '1'
20
+                                         })
21
+
20 22
     @checker.user = users(:bob)
21 23
     @checker.save!
22 24
 
23 25
     @event = Event.new
24 26
     @event.agent = agents(:bob_weather_agent)
25
-    @event.payload = { :message => 'Looks like its going to rain' }
27
+    @event.payload = { message: 'Looks like its going to rain' }
26 28
     @event.save!
27 29
 
28 30
     @sent_notifications = []
@@ -33,12 +35,12 @@ describe Agents::PushoverAgent do
33 35
     it 'should make sure multiple events are being received' do
34 36
       event1 = Event.new
35 37
       event1.agent = agents(:bob_rain_notifier_agent)
36
-      event1.payload = { :message => 'Some message' }
38
+      event1.payload = { message: 'Some message' }
37 39
       event1.save!
38 40
 
39 41
       event2 = Event.new
40 42
       event2.agent = agents(:bob_weather_agent)
41
-      event2.payload = { :message => 'Some other message' }
43
+      event2.payload = { message: 'Some other message' }
42 44
       event2.save!
43 45
 
44 46
       @checker.receive([@event,event1,event2])
@@ -50,7 +52,7 @@ describe Agents::PushoverAgent do
50 52
     it 'should make sure event message overrides default message' do
51 53
       event = Event.new
52 54
       event.agent = agents(:bob_rain_notifier_agent)
53
-      event.payload = { :message => 'Some new message'}
55
+      event.payload = { message: 'Some new message'}
54 56
       event.save!
55 57
 
56 58
       @checker.receive([event])
@@ -60,7 +62,7 @@ describe Agents::PushoverAgent do
60 62
     it 'should make sure event text overrides default message' do
61 63
       event = Event.new
62 64
       event.agent = agents(:bob_rain_notifier_agent)
63
-      event.payload = { :text => 'Some new text'}
65
+      event.payload = { text: 'Some new text'}
64 66
       event.save!
65 67
 
66 68
       @checker.receive([event])
@@ -70,7 +72,7 @@ describe Agents::PushoverAgent do
70 72
     it 'should make sure event title overrides default title' do
71 73
       event = Event.new
72 74
       event.agent = agents(:bob_rain_notifier_agent)
73
-      event.payload = { :message => 'Some message', :title => 'Some new title' }
75
+      event.payload = { message: 'Some message', title: 'Some new title' }
74 76
       event.save!
75 77
 
76 78
       @checker.receive([event])
@@ -80,7 +82,7 @@ describe Agents::PushoverAgent do
80 82
     it 'should make sure event url overrides default url' do
81 83
       event = Event.new
82 84
       event.agent = agents(:bob_rain_notifier_agent)
83
-      event.payload = { :message => 'Some message', :url => 'Some new url' }
85
+      event.payload = { message: 'Some message', url: 'Some new url' }
84 86
       event.save!
85 87
 
86 88
       @checker.receive([event])
@@ -90,7 +92,7 @@ describe Agents::PushoverAgent do
90 92
     it 'should make sure event url_title overrides default url_title' do
91 93
       event = Event.new
92 94
       event.agent = agents(:bob_rain_notifier_agent)
93
-      event.payload = { :message => 'Some message', :url_title => 'Some new url_title' }
95
+      event.payload = { message: 'Some message', url_title: 'Some new url_title' }
94 96
       event.save!
95 97
 
96 98
       @checker.receive([event])
@@ -100,17 +102,17 @@ describe Agents::PushoverAgent do
100 102
     it 'should make sure event priority overrides default priority' do
101 103
       event = Event.new
102 104
       event.agent = agents(:bob_rain_notifier_agent)
103
-      event.payload = { :message => 'Some message', :priority => 1 }
105
+      event.payload = { message: 'Some message', priority: '1' }
104 106
       event.save!
105 107
 
106 108
       @checker.receive([event])
107
-      expect(@sent_notifications[0]['priority']).to eq(1)
109
+      expect(@sent_notifications[0]['priority']).to eq('1')
108 110
     end
109 111
 
110 112
     it 'should make sure event timestamp overrides default timestamp' do
111 113
       event = Event.new
112 114
       event.agent = agents(:bob_rain_notifier_agent)
113
-      event.payload = { :message => 'Some message', :timestamp => 'false' }
115
+      event.payload = { message: 'Some message', timestamp: 'false' }
114 116
       event.save!
115 117
 
116 118
       @checker.receive([event])
@@ -120,7 +122,7 @@ describe Agents::PushoverAgent do
120 122
     it 'should make sure event sound overrides default sound' do
121 123
       event = Event.new
122 124
       event.agent = agents(:bob_rain_notifier_agent)
123
-      event.payload = { :message => 'Some message', :sound => 'Some new sound' }
125
+      event.payload = { message: 'Some message', sound: 'Some new sound' }
124 126
       event.save!
125 127
 
126 128
       @checker.receive([event])
@@ -130,21 +132,21 @@ describe Agents::PushoverAgent do
130 132
     it 'should make sure event retry overrides default retry' do
131 133
       event = Event.new
132 134
       event.agent = agents(:bob_rain_notifier_agent)
133
-      event.payload = { :message => 'Some message', :retry => 1 }
135
+      event.payload = { message: 'Some message', retry: 1 }
134 136
       event.save!
135 137
 
136 138
       @checker.receive([event])
137
-      expect(@sent_notifications[0]['retry']).to eq(1)
139
+      expect(@sent_notifications[0]['retry']).to eq('1')
138 140
     end
139 141
 
140 142
     it 'should make sure event expire overrides default expire' do
141 143
       event = Event.new
142 144
       event.agent = agents(:bob_rain_notifier_agent)
143
-      event.payload = { :message => 'Some message', :expire => 60 }
145
+      event.payload = { message: 'Some message', expire: '60' }
144 146
       event.save!
145 147
 
146 148
       @checker.receive([event])
147
-      expect(@sent_notifications[0]['expire']).to eq(60)
149
+      expect(@sent_notifications[0]['expire']).to eq('60')
148 150
     end
149 151
   end
150 152
 
@@ -158,7 +160,7 @@ describe Agents::PushoverAgent do
158 160
       expect(@checker.reload).to be_working
159 161
       two_days_from_now = 2.days.from_now
160 162
       stub(Time).now { two_days_from_now }
161
-      
163
+
162 164
       # More time has passed than the expected receive period without any new events
163 165
       expect(@checker.reload).not_to be_working
164 166
     end